Skip to content

feat(spec): raise completeness and quality of /awos:spec output#140

Open
FlySpot wants to merge 1 commit into
mainfrom
spec-command-optimization
Open

feat(spec): raise completeness and quality of /awos:spec output#140
FlySpot wants to merge 1 commit into
mainfrom
spec-command-optimization

Conversation

@FlySpot

@FlySpot FlySpot commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

What

Applies a set of targeted improvements to `/awos:spec` that close five quality gaps identified in the optimization plan. All changes were validated end-to-end in the awos-qa fork before being ported here.

Changes to `commands/spec.md`

Step 3 — Functional Requirements & Acceptance Criteria (Points 2)

  • New bullet: "Where applicable, capture the boundary and error behavior the user sees: what error message appears, what limits exist (file size, format, count), and what happens when the action fails."
  • New bullet: "For requirements that capture boundary or error behavior, include at least one acceptance criterion covering the failure path."

Step 4 — Self-Review (Point 4)

  • Renamed to "Self-Review (Language and Ambiguity Check)"
  • Added a second pass that scans §Functional Requirements for vague or unmeasurable wording (weasel words like "fast", "user-friendly", "as appropriate") and requires each to be made concrete in user-perceivable terms or converted to a [NEEDS CLARIFICATION: …] marker.

Step 5 — Specification Definition of Done (Points 1, 5, 7) — new step

  • Pre-write self-check (not a gate — file is always written):
    1. No vague wording remains in requirements or acceptance criteria.
    2. Every requirement has at least one When/Then acceptance criterion.
    3. No unresolved [NEEDS CLARIFICATION: …] markers. In unattended runs, each marker is replaced with a visible **Assumption:** … line so open decisions are recorded rather than hidden.
  • Old Step 5 (Final Review) → Step 6; old Step 6 (File Generation) → Step 7.

New file: `docs/optimization/spec-command-optimization.md`

Documents the problem statement, fix direction, and placement for all seven optimization points, including the rationale for carving out Points 3 (non-functional requirements) and 6 (relative-importance signal) as separate cross-command tasks.

Why

Before this change:

  • Error and boundary conditions appeared only in an example question, not as a rule — requirements routinely omitted failure paths.
  • The self-review caught technical language leakage but not vague, untestable wording like "user-friendly" or "as appropriate".
  • No pre-write gate verified that every requirement had a criterion, or that all [NEEDS CLARIFICATION] markers were resolved.

Out of scope (separate tasks)

  • Point 3 — Non-functional requirements with measurable metrics. These belong in `technical-considerations.md`; adding them to the functional spec breaks the what/how boundary. Requires coordinated changes to `tech.md` and `technical-considerations-template.md`.
  • Point 6 — Per-requirement importance signal. No current consumer exists in `tasks.md`, `tech.md`, or `verify.md`; adding the signal without consumption is dead state. Requires coordinated changes to `functional-spec-template.md` and `tasks.md`.

Tests

Three new lint checks in `tests/lint-prompts.test.js` pin the contracts so they cannot be silently dropped:

  • Definition of Done checklist presence (anchors: Definition of Done, **Assumption:**, at least one acceptance criterion, No vague wording remains)
  • Ambiguity self-review presence (vague or unmeasurable wording, "user-friendly", concrete in user-perceivable terms)
  • Boundary/error rules presence (boundary and error behavior the user sees, at least one acceptance criterion covering the failure path)

Behavioral coverage (no residual markers in produced `functional-spec.md`, every requirement carries a criterion, no unverifiable wording) is in the awos-qa repository.

Summary by CodeRabbit

  • Documentation

    • Updated the functional-spec writing guide to require explicit user-visible boundary and error behavior, including failure-path acceptance criteria.
    • Revised the self-review step to include an added language/ambiguity check and a stronger “Definition of Done” checklist to remove vague wording.
    • Expanded the final review guidance to ensure all unresolved clarifications are addressed before completion.
  • Tests

    • Added automated lint checks to confirm the updated spec guidance and review contracts are present and followed.

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fb3d4244-8507-4725-85e6-ded8dfbb2588

📥 Commits

Reviewing files that changed from the base of the PR and between 840cd39 and 59f9fce.

📒 Files selected for processing (2)
  • commands/spec.md
  • tests/lint-prompts.test.js
✅ Files skipped from review due to trivial changes (1)
  • commands/spec.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/lint-prompts.test.js

📝 Walkthrough

Walkthrough

This PR updates the /awos:spec guide to require boundary/error behavior in Functional Requirements, resolve clarification markers before Acceptance Criteria, add a language-and-ambiguity self-review, and define a stricter Definition of Done. Matching lint tests were added.

Changes

Spec Prompt Guidance

Layer / File(s) Summary
Boundary and failure-path rules
commands/spec.md, tests/lint-prompts.test.js
Functional Requirements now include user-visible boundary/error behavior; Acceptance Criteria resolve clarification markers first and require failure-path coverage; lint tests assert the new boundary/error prompt text.
Ambiguity check and Definition of Done
commands/spec.md, tests/lint-prompts.test.js
Self-Review becomes a language and ambiguity check; a Definition of Done checklist adds vague-wording and acceptance-criteria checks; final review clarifies remaining markers with AskUserQuestion; lint tests verify the prompt contract.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • provectus/awos#113: Also tightens /awos:spec acceptance-criteria and clarification handling rules.
  • provectus/awos#116: Also changes clarification and language-check behavior in commands/spec.md.
  • provectus/awos#141: Also updates the spec workflow around unresolved clarification markers and prompt handling.

Suggested reviewers: AlexanderMakarov

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: improving the completeness and quality of /awos:spec output.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch spec-command-optimization

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@FlySpot FlySpot requested a review from AlexanderMakarov June 25, 2026 11:06

@AlexanderMakarov AlexanderMakarov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tightens /awos:spec in sensible places: it makes boundary/error behavior an explicit elicitation rule rather than a buried example, requires a failure-path acceptance criterion for those requirements, adds a weasel-word pass to the self-review, and closes the loop with a pre-write Definition of Done that sweeps [NEEDS CLARIFICATION] markers. The prompt edits read clean — the Step 4↔Step 5 cross-references line up, the marker-sweep "runs last so it also catches markers from items 1 and 2" ordering is thought through, the old-Step-5/6 → 6/7 renumbering is applied consistently, and the wording stays plain-declarative (one bold emphasis per rule) per the repo's emphasis convention. Nothing in the actual command blocks merge.

What is docs/optimization/spec-command-optimization.md for? It's a new pattern (no docs/optimization/ today) and it isn't shipped by the installer, so this is purely about the repo's own docs. After merge, five of its seven points (1, 2, 4, 5, 7) are implemented — the doc then describes problems that no longer exist, in the present tense. The repo's own rule is "docs describe the present, not the history." The genuinely live content is Points 3 and 6 (the deferred cross-command work). Worth deciding: keep it as a durable record (then it wants section/step references instead of line numbers, and past-tense problem statements), or trim it to the deferred tasks and let the PR description carry the rest? Your call — I just don't want it to rot silently.

Comment thread tests/lint-prompts.test.js Outdated
'commands/spec.md Definition of Done must tell unattended runs to replace each unresolved [NEEDS CLARIFICATION] marker with a visible **Assumption:** and still save the file'
);
assert.ok(
/at least one acceptance criterion/i.test(body),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion is meant to pin DoD item 2 ("Every requirement has at least one acceptance criterion"), but /at least one acceptance criterion/i also matches the Step 3 failure-path rule you added at spec.md:99 ("include at least one acceptance criterion covering the failure path"). So if someone deleted the DoD completeness gate but left the Step 3 rule, this stays green — the item it names isn't actually covered. The other three assertions in this test (Definition of Done, No vague wording remains, **Assumption:**) are unique, so the section as a whole is pinned; it's just this one item that slips through. Anchoring on the unique bolded lead-in would fix it:

assert.ok(
  body.includes('Every requirement has at least one acceptance criterion'),
  '...'
);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed. /at least one acceptance criterion/i also matched the Step 3 failure-path rule at spec.md:99, so DoD item 2 was not pinned — removing line 118 left the test green. The assertion now reads body.includes('Every requirement has at least one acceptance criterion'), which occurs only at the DoD line. Verified by mutation: deleting DoD item 2 now turns the test red. The Step 3 failure-path rule stays independently pinned by the boundary test.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed at head — the exact substring occurs once, only at the DoD item 2 line. The mutation check is appreciated.

Comment thread tests/lint-prompts.test.js Outdated
'commands/spec.md self-review must name concrete weasel-word examples (e.g. "user-friendly") so the model knows what to hunt'
);
assert.ok(
/concrete in user-perceivable terms/i.test(body),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, optional — same flavor as the DoD one but not load-bearing: /concrete in user-perceivable terms/i matches both the Step 4 pass (spec.md:111) and DoD item 1 (spec.md:117), so on its own it wouldn't catch Step 4 being removed. The sibling assertion /vague or unmeasurable wording/i is unique to Step 4, so the test still does its job. Leave it or tighten to a Step-4-only phrase (e.g. the search results appear within 2 seconds example) — your preference.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tightened. The assertion now anchors on the Step 4 rule sentence, body.includes('Make each one concrete in user-perceivable terms'), unique to line 111. The example phrase was not used as the anchor: it would leave the rule deletable while the test stayed green, whereas the rule sentence pins what the assertion message describes. Verified by mutation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed — unique to the Step 4 rule sentence at head. And agreed the example phrase would have been the weaker anchor: the rule sentence is what the assertion message names.


## Point 1 — Unresolved `[NEEDS CLARIFICATION]` markers reaching the saved file

- **Problem:** The marker is only required to be closed on the parent requirement before acceptance criteria are written — `commands/spec.md:96`: _"If any `[NEEDS CLARIFICATION: …]` markers remain on the parent requirement in §Functional Requirements, ask clarifying questions and resolve the markers before writing acceptance criteria."_ Neither the self-review (Step 4, line 108) nor file generation (Step 6, lines 114–119) blocks saving a file with markers still open in other sections. The marker is introduced at `commands/spec.md:90`: _"mark every unresolved detail with `[NEEDS CLARIFICATION: your specific question]`"_.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commands/spec.md:NN citations here are accurate against main as it stands, but this PR's own edits shift every one of them down (~+2 lines) and renumber the steps — so on merge they point at the wrong lines, and "file generation (Step 6, lines 114–119)" names a step that's now Step 7. Same root shows in Point 4 (line 30): "the self-review catches only leakage … weasel words [are] not present in spec.md today" reads in the present tense, but this PR is exactly what makes that false. If the doc stays, citing by section/step name and writing the problem statements in past tense ("before this change…") keeps it from contradicting the file it ships with.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved by removing the document. The stale spec.md:NN citations and the present-tense "not present today" statement are removed with it.

Comment thread commands/spec.md Outdated
### Step 4: Self-Review (Language and Ambiguity Check)

- Before presenting to the user, re-read the entire draft end-to-end. For every sentence, ask: "Would this make sense to someone who has never seen the codebase?" Replace any developer-facing language with plain, non-technical wording in the same language the user is using. Remove any references to internal system behavior, code, or architecture that slipped in.
- Then re-read §Functional Requirements — both the requirement statements and their acceptance criteria — for vague or unmeasurable wording: words like "fast", "user-friendly", or "as appropriate" that a tester could not verify. Make each one concrete in user-perceivable terms (e.g., "the search feels fast" becomes "search results appear within 2 seconds"), or, if the user has not decided the specific value yet, replace it with a `[NEEDS CLARIFICATION: …]` marker so it is resolved in Step 5. Quantify only where the user would notice the difference — do not force a number onto every sentence; narrative sections like §Overview and Rationale may stay qualitative.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like §Overview and Rationale may stay qualitative.

Weak reference - I don't understand where name of section ends. The same for re-read §Functional Requirements — both.

Please use [Link Text](#section-name) formatting. Here and below in this file.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The section references are disambiguated: all five § tokens are replaced with the bold section name (e.g. §Functional RequirementsFunctional Requirements), applied throughout the file.

[Link Text](#section-name) was not used because these references point to sections of the generated functional-spec.md and its template, not to headings within spec.md; an anchor in the prompt file would resolve to no target. Bold delimiting removes the ambiguity without a broken link.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified at head — zero § tokens remain. And you're right about the anchor links: these are headings of the generated spec (templates/functional-spec-template.md lines 9 and 15), not headings in this file, so (#section-name) would have been a broken link. Bold is the right call.

Comment thread commands/spec.md Outdated

Before the final review and saving the file, check the draft against this list. It is a self-review, not a new approval gate — the file is still written at the end of the process.

1. **No vague wording remains in requirements or acceptance criteria.** Each requirement and criterion is concrete enough that a non-developer can tell unambiguously whether it passes or fails. If a vague term slipped through, resolve it as described in Step 4: make it concrete in user-perceivable terms, or convert it to a `[NEEDS CLARIFICATION: …]` marker — the marker is a temporary hand-off, not a final state; item 3 closes every marker before the file is written.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No vague wording remains in requirements or acceptance criteria.

I guess it is excessive - big part of step 4 was dedicated to this check. If we make LLM to check itself in the same session (the same as asking person "Are you done?") it rarely works - LLM reads from it's own context that everything is completed already and do nothing (as the person answers "Yes" without checking, from memory). So this usually just a waste of tokens.

We may ask it to Grep in the specific file specific word instead - item 3 below.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Item 1 now defers to Step 4 — a one-clause recap plus a cross-reference — instead of restating the check as an independent rule. A Grep was not adopted, for two reasons: the Definition of Done runs at Step 5, before file generation at Step 7, so no file exists on disk to grep — the whole checklist, including the marker sweep, operates on the in-context draft; and vagueness is not a fixed token set, so grepping the named example words would miss other wording.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deferral reads well — item 1 is now a hand-off to Step 4 rather than a second full pass, and fair pushback on the Grep: at this step nothing is on disk yet, so there's nothing to grep. One heads-up: the checklist's position is affected by the #141 rebase (see the review summary) — items 1–2 can stay pre-write as they are; item 3 is the one that has to move.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed. Definition-of-Done items 1–2 (no vague wording remains; every requirement carries at least one acceptance criterion) remain pre-write self-checks in Step 4, with no user interaction. Item 3's interactive part is relocated to Step 6: post-save resolution of each [NEEDS CLARIFICATION] marker via AskUserQuestion offers the default assumption as the recommended first option (with a free-text option for open-ended markers), then folds the answer back into the requirement and its acceptance criteria. Main's write-first ordering is preserved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed at head — items 1–2 are pre-write self-checks in Step 4 with no user interaction, and item 3's interactive part now lives in post-save Step 6. That's the split I was after, and the write-first ordering holds.

Comment thread commands/spec.md Outdated

1. **No vague wording remains in requirements or acceptance criteria.** Each requirement and criterion is concrete enough that a non-developer can tell unambiguously whether it passes or fails. If a vague term slipped through, resolve it as described in Step 4: make it concrete in user-perceivable terms, or convert it to a `[NEEDS CLARIFICATION: …]` marker — the marker is a temporary hand-off, not a final state; item 3 closes every marker before the file is written.
2. **Every requirement has at least one acceptance criterion.** Confirm that each functional requirement in §Functional Requirements carries at least one acceptance criterion in the When/Then shape (Given optional). If a requirement has none, write one for it before saving.
3. **No unresolved clarification markers.** Scan the whole document for `[NEEDS CLARIFICATION: …]` markers, not only the section you edited last — this check runs last so it also catches markers created by items 1 and 2. In an interactive session, resolve each one with the user and fold the answer into the relevant requirement and its acceptance criteria. In an unattended session where no one can answer, **do not stop**: replace each marker with a visible `**Assumption:** …` line in plain language and save the file anyway, so the open decision is recorded rather than hidden. Example: `The user sees an error message. [NEEDS CLARIFICATION: exact wording?]` becomes `The user sees an error message. **Assumption:** the message reads "Something went wrong — please try again."`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In an interactive session, resolve each one with the user

Please be more precise - like using AskUserQuestion tool with reasonable defaults - it would make LLM understand (and, therefore, implement) task better and be proactive for user (don't invent units for metrics).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. The interactive branch of item 3 now resolves each marker with the user via AskUserQuestion, offering the assumption it would otherwise make as the recommended first option, with a free-text option for open-ended markers. It remains a confirmable choice rather than an auto-accept, and the unattended branch (**do not stop** / **Assumption:**) is unchanged, so the deliverable is still never gated on an answer. Metric units are not invented: the model proposes a value for the user to confirm, or records an explicit assumption when unattended.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AskUserQuestion mechanics are what I was after — the assumption as the recommended first option is exactly right. Heads-up though: after rebasing onto #141, this interactive branch can't stay ahead of the file write — in claude -p a dismissed AskUserQuestion ends the turn before the Write ever runs, which is the exact failure #141 restructured this file around. Main's Step 6 now does post-save marker resolution; I'd move these mechanics there. Details in the review summary.

@oleksii-shevchuk oleksii-shevchuk Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed. The **Assumption:** fallback is removed. An unattended run now leaves the [NEEDS CLARIFICATION] marker in the saved file, per main's contract pinned by the deliverable commands write their file on an unanswered question test. Markers are no longer replaced with assumptions before saving, which aligns spec.md with product/roadmap and with Step 6.
Concretely: the AskUserQuestion marker-resolution now lives in post-save Step 6, after the file is written in Step 5 — no dismissable question precedes the Write, so the claude -p failure mode #141 restructured around is gone.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed — the AskUserQuestion resolution now sits in Step 6 after the Step 5 write, and the unattended path leaves the marker in the saved file per main's contract. No dismissable question precedes the Write anymore, so the claude -p failure mode is closed.

@@ -0,0 +1,51 @@
# Spec Command Optimization

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TL;DR: better to move this file into separate PR. I don't see how it is used here.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The document is removed from this PR. The deferred points (Non-Functional Metrics; Requirement-Priority Signal) will be addressed in their own coordinated PRs.

@FlySpot FlySpot requested a review from oleksii-shevchuk July 6, 2026 13:02
@oleksii-shevchuk

Copy link
Copy Markdown
Collaborator

All review points are addressed in the latest revision — specifics are in each thread. No load-bearing contract changed: the Definition of Done stays a self-review, the file is always written, and the unattended assumption path is intact.

@AlexanderMakarov AlexanderMakarov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round 2 — everything from the first pass is addressed — thanks for the careful replies.

One thing now stands between this and merge, and it isn't a defect in this PR: main moved underneath it. #141 restructured commands/spec.md around write-first delivery — the file is written at Step 5 (File Generation) before any final review, and Step 6 resolves [NEEDS CLARIFICATION] markers with the user after the save. Its editor note explains why: in claude -p, a dismissed AskUserQuestion ends the turn, so a deliverable Write placed after a dismissable question never runs unattended. The branch is now conflicting in both files, and the resolution is semantic, not textual. Two decisions it forces:

  1. Where the Definition of Done lives. Items 1–2 (vague-wording confirm, criterion coverage) are pure self-checks with no user interaction — they can stay ahead of File Generation as they are. Item 3's interactive branch, though, puts an AskUserQuestion ahead of the deliverable Write — exactly the structure #141 exists to prevent. I'd fold item 3's mechanics (offer the assumption you'd otherwise make as the recommended first option) into main's Step 6, which already does post-save marker resolution with the user. That keeps your improvement and main's ordering.

  2. What an unattended run leaves in the file. Main's contract — codified in deliverable commands write their file on an unanswered question — is that spec.md's unattended fallback is the [NEEDS CLARIFICATION: …] marker itself, left in the saved file for the user or /awos:tech to resolve later. This PR replaces markers with **Assumption:** … lines before saving. Both record the open decision, but they can't both be the story. I'd keep main's markers — that's the merged, test-pinned contract and the smaller delta. (I checked: nothing greps for the marker outside commands/spec.md today — tech.md has no marker consumer — so this is about keeping one documented hand-off, not protecting a live consumer. Which also means switching to **Assumption:** is viable if you think a recorded assumption is strictly better than an open marker; it just has to be one story, with #141's test comment and the INTERACTION rule updated in the same PR rather than the two drifting apart.)

Comment thread commands/spec.md Outdated
### Step 4: Self-Review (Language and Ambiguity Check)

- Before presenting to the user, re-read the entire draft end-to-end. For every sentence, ask: "Would this make sense to someone who has never seen the codebase?" Replace any developer-facing language with plain, non-technical wording in the same language the user is using. Remove any references to internal system behavior, code, or architecture that slipped in.
- Then re-read §Functional Requirements — both the requirement statements and their acceptance criteria — for vague or unmeasurable wording: words like "fast", "user-friendly", or "as appropriate" that a tester could not verify. Make each one concrete in user-perceivable terms (e.g., "the search feels fast" becomes "search results appear within 2 seconds"), or, if the user has not decided the specific value yet, replace it with a `[NEEDS CLARIFICATION: …]` marker so it is resolved in Step 5. Quantify only where the user would notice the difference — do not force a number onto every sentence; narrative sections like §Overview and Rationale may stay qualitative.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified at head — zero § tokens remain. And you're right about the anchor links: these are headings of the generated spec (templates/functional-spec-template.md lines 9 and 15), not headings in this file, so (#section-name) would have been a broken link. Bold is the right call.

Comment thread commands/spec.md Outdated

Before the final review and saving the file, check the draft against this list. It is a self-review, not a new approval gate — the file is still written at the end of the process.

1. **No vague wording remains in requirements or acceptance criteria.** Each requirement and criterion is concrete enough that a non-developer can tell unambiguously whether it passes or fails. If a vague term slipped through, resolve it as described in Step 4: make it concrete in user-perceivable terms, or convert it to a `[NEEDS CLARIFICATION: …]` marker — the marker is a temporary hand-off, not a final state; item 3 closes every marker before the file is written.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deferral reads well — item 1 is now a hand-off to Step 4 rather than a second full pass, and fair pushback on the Grep: at this step nothing is on disk yet, so there's nothing to grep. One heads-up: the checklist's position is affected by the #141 rebase (see the review summary) — items 1–2 can stay pre-write as they are; item 3 is the one that has to move.

Comment thread commands/spec.md Outdated

1. **No vague wording remains in requirements or acceptance criteria.** Each requirement and criterion is concrete enough that a non-developer can tell unambiguously whether it passes or fails. If a vague term slipped through, resolve it as described in Step 4: make it concrete in user-perceivable terms, or convert it to a `[NEEDS CLARIFICATION: …]` marker — the marker is a temporary hand-off, not a final state; item 3 closes every marker before the file is written.
2. **Every requirement has at least one acceptance criterion.** Confirm that each functional requirement in §Functional Requirements carries at least one acceptance criterion in the When/Then shape (Given optional). If a requirement has none, write one for it before saving.
3. **No unresolved clarification markers.** Scan the whole document for `[NEEDS CLARIFICATION: …]` markers, not only the section you edited last — this check runs last so it also catches markers created by items 1 and 2. In an interactive session, resolve each one with the user and fold the answer into the relevant requirement and its acceptance criteria. In an unattended session where no one can answer, **do not stop**: replace each marker with a visible `**Assumption:** …` line in plain language and save the file anyway, so the open decision is recorded rather than hidden. Example: `The user sees an error message. [NEEDS CLARIFICATION: exact wording?]` becomes `The user sees an error message. **Assumption:** the message reads "Something went wrong — please try again."`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AskUserQuestion mechanics are what I was after — the assumption as the recommended first option is exactly right. Heads-up though: after rebasing onto #141, this interactive branch can't stay ahead of the file write — in claude -p a dismissed AskUserQuestion ends the turn before the Write ever runs, which is the exact failure #141 restructured this file around. Main's Step 6 now does post-save marker resolution; I'd move these mechanics there. Details in the review summary.

Comment thread tests/lint-prompts.test.js Outdated
'commands/spec.md Definition of Done must tell unattended runs to replace each unresolved [NEEDS CLARIFICATION] marker with a visible **Assumption:** and still save the file'
);
assert.ok(
/at least one acceptance criterion/i.test(body),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed at head — the exact substring occurs once, only at the DoD item 2 line. The mutation check is appreciated.

Comment thread tests/lint-prompts.test.js Outdated
'commands/spec.md self-review must name concrete weasel-word examples (e.g. "user-friendly") so the model knows what to hunt'
);
assert.ok(
/concrete in user-perceivable terms/i.test(body),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed — unique to the Step 4 rule sentence at head. And agreed the example phrase would have been the weaker anchor: the rule sentence is what the assertion message names.

Layer four improvements onto the write-first structure from #141:
- Step 3: elicit the boundary/error behavior the user sees, and require
  at least one acceptance criterion covering the failure path.
- Step 4: add a Language and Ambiguity self-review that turns vague,
  unmeasurable wording ("fast", "user-friendly") into user-perceivable
  terms or a [NEEDS CLARIFICATION] marker, plus a Definition of Done
  sub-block (no vague wording remains; every requirement carries a
  criterion) — self-checks only, with no user turn before the write.
- Step 6: when resolving each [NEEDS CLARIFICATION] marker post-save,
  offer the assumption the model would otherwise make as the recommended
  first AskUserQuestion option.

The file is still written first (Step 5), and an unattended run leaves
the [NEEDS CLARIFICATION] marker in place per main's deliverable-write
contract — no **Assumption:** substitution before saving. Cross-section
references use bold names instead of the ambiguous § symbol.

Add three Layer-1 lint tests pinning the Definition of Done, the
ambiguity check, and the boundary/error rules.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Alex Malakhov <amalakhovod@gmail.com>
Co-Authored-By: Pavlo Samoilov <psamoilov@provectus.com>
@oleksii-shevchuk oleksii-shevchuk force-pushed the spec-command-optimization branch from 840cd39 to 59f9fce Compare July 7, 2026 23:51
@oleksii-shevchuk

Copy link
Copy Markdown
Collaborator

Round 2 addressed.
Since main moved (#141, write-first), this is a semantic reconciliation rather than a textual merge; the branch is rebased onto origin/main.

  • The Step 4/5/6 skeleton and write-first File Generation match main.
  • Four improvements are preserved: boundary/error elicitation and a failure-path acceptance criterion (Step 3); the vague/unmeasurable-wording check and the Definition-of-Done sub-block (Step 4).
  • Interactive marker resolution is moved to post-save Step 6.
  • The **Assumption:** fallback is dropped in favor of main's marker contract.

Tests: the deliverable… contract is kept verbatim, a lint anchor for the Step 6 mechanic is added, and the stale comments are updated.

@AlexanderMakarov AlexanderMakarov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round 3 — this is all addressed — thanks for the careful semantic merge.

One housekeeping thing before you squash-merge — not a code issue, but the description lands in the commit history and currently describes a design that didn't ship. Three spots:

  1. The "Step 5 — Specification Definition of Done … new step" section and the "Old Step 5 (Final Review) → Step 6; old Step 6 (File Generation) → Step 7" renumbering. At head there's no Step 7 and no standalone DoD step: the Definition of Done lives inside Step 4, Step 5 is File Generation, Step 6 is Final Review.
  2. DoD item 3's "each marker is replaced with a visible **Assumption:** … line". That fallback was dropped — unattended runs now leave the [NEEDS CLARIFICATION: …] marker in the saved file, per main's contract.
  3. The Tests section lists **Assumption:** as a Definition-of-Done test anchor; the test doesn't assert on it — its fourth anchor is offering the assumption you would otherwise make as the recommended first option.

Refresh those and I'm good to merge.

Comment thread commands/spec.md Outdated

Before the final review and saving the file, check the draft against this list. It is a self-review, not a new approval gate — the file is still written at the end of the process.

1. **No vague wording remains in requirements or acceptance criteria.** Each requirement and criterion is concrete enough that a non-developer can tell unambiguously whether it passes or fails. If a vague term slipped through, resolve it as described in Step 4: make it concrete in user-perceivable terms, or convert it to a `[NEEDS CLARIFICATION: …]` marker — the marker is a temporary hand-off, not a final state; item 3 closes every marker before the file is written.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed at head — items 1–2 are pre-write self-checks in Step 4 with no user interaction, and item 3's interactive part now lives in post-save Step 6. That's the split I was after, and the write-first ordering holds.

Comment thread commands/spec.md Outdated

1. **No vague wording remains in requirements or acceptance criteria.** Each requirement and criterion is concrete enough that a non-developer can tell unambiguously whether it passes or fails. If a vague term slipped through, resolve it as described in Step 4: make it concrete in user-perceivable terms, or convert it to a `[NEEDS CLARIFICATION: …]` marker — the marker is a temporary hand-off, not a final state; item 3 closes every marker before the file is written.
2. **Every requirement has at least one acceptance criterion.** Confirm that each functional requirement in §Functional Requirements carries at least one acceptance criterion in the When/Then shape (Given optional). If a requirement has none, write one for it before saving.
3. **No unresolved clarification markers.** Scan the whole document for `[NEEDS CLARIFICATION: …]` markers, not only the section you edited last — this check runs last so it also catches markers created by items 1 and 2. In an interactive session, resolve each one with the user and fold the answer into the relevant requirement and its acceptance criteria. In an unattended session where no one can answer, **do not stop**: replace each marker with a visible `**Assumption:** …` line in plain language and save the file anyway, so the open decision is recorded rather than hidden. Example: `The user sees an error message. [NEEDS CLARIFICATION: exact wording?]` becomes `The user sees an error message. **Assumption:** the message reads "Something went wrong — please try again."`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed — the AskUserQuestion resolution now sits in Step 6 after the Step 5 write, and the unattended path leaves the marker in the saved file per main's contract. No dismissable question precedes the Write anymore, so the claude -p failure mode is closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants